Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Maps] Delay vector tile layer syncing until spritesheet is loaded #48955

Merged
merged 5 commits into from
Nov 18, 2019

Conversation

thomasneirynck
Copy link
Contributor

Delay bootstrapping of the layer until the spritesheet imagedata is loaded.

This should close a race condition, and likely (?) address #48933

@thomasneirynck thomasneirynck added bug Fixes for quality problems that affect the customer experience release_note:fix [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation labels Oct 22, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@thomasneirynck thomasneirynck added WIP Work in progress and removed bug Fixes for quality problems that affect the customer experience labels Oct 22, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Water waves appear in both Chrome and Firefox with this PR. Prior to this, the waves would not appear in Chrome for me. Code changes look good.

}
}

export async function addSpritesheetToMap(json, imgUrl, mbMap) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, maybe we should standardize the capitalization of SpriteSheet in our function names?

Suggested change
export async function addSpritesheetToMap(json, imgUrl, mbMap) {
export async function addSpriteSheetToMap(json, imgUrl, mbMap) {

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the improvement of moving the loading into syncData but I am not sure this fixes the problem. While running this branch, the waves still failed to load.

Screen Shot 2019-10-23 at 4 00 58 PM

@thomasneirynck
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck
Copy link
Contributor Author

@nreese bummer that this doesn't seem to fix that race-condition in all cases. Would still like to go forward with this PR, as this is already an improvement and fixes a known issue.

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@thomasneirynck thomasneirynck removed the WIP Work in progress label Nov 15, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck merged commit 929384b into elastic:master Nov 18, 2019
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Nov 18, 2019
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Nov 18, 2019
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 18, 2019
…-fallback

* 'master' of github.com:elastic/kibana: (116 commits)
  [Maps] move apply global filter settting from layer to source (elastic#50523)
  [SIEM] Fix: Empty `Source` / `Destination` shown when only ports are populated (elastic#50843)
  [Maps] Delay vector tile layer syncing until spritesheet is loaded (elastic#48955)
  [Maps] prevent users from overflowing URL when filtering by shape (elastic#50747)
  [DOCS] Mark Beats central management as discontinued (elastic#49423)
  [page_objects/common_page] convert to ts (elastic#50771)
  [NP Kibana Migrations ] kibana plugin home (elastic#50444)
  [DOCS] Shareables naming convention (elastic#50497)
  [ML] DF Analytics - auto-populate model_memory_limit (elastic#50714)
  Increase alerting test stability and reduce flakiness (elastic#50246)
  [ML] Remaning new_job_new folder (elastic#50917)
  [Telemetry] Show opt-in changes for OSS users (elastic#50831)
  [ML] Fix lat_long anomalies table links menu and value formatting (elastic#50916)
  [Dev] Fix serialising a really big string (elastic#50915)
  Better explanation about the Prettier recommendation (extension vs. NPM module) (elastic#50629)
  [Monitoring] Use a basic monitoring user for tests (elastic#47865)
  [Monitoring] Gracefully handle issue with filebeat indices (elastic#48929)
  [Monitoring] Improve permissions required around setup mode (elastic#50421)
  Additional validation for elasticsearch username (elastic#48247)
  Revert changes to use_kibana_ui_setting (elastic#50877)
  ...

# Conflicts:
#	src/legacy/core_plugins/console/server/request.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants